Teleport Connect headless approval - Skip Confirmation by Joerger · Pull Request #29875 · gravitational/teleport

Joerger · 2023-08-01T18:55:42Z

Updates Teleport Connect headless approval flow to skip the confirmation step (Approve/Cancel) and go straight to prompting for MFA in the background when the headless.skipConfirm config option is set to true.

Docs will be added in a follow up PR.

Joerger · 2023-08-01T18:57:27Z

@gzdunek @ravicious Any idea why I am getting process is not defined errors? Shouldn't process.env be accessible globally?

gzdunek · 2023-08-02T08:33:20Z

Shouldn't process.env be accessible globally?

No, it is a Node.js thing, and it is not passed to the renderer (or, in other words, the browser window).
There are ways to pass a value from the main process to the renderer, for example via our RuntimeSettings, but I'm wondering if it is a best idea to configure the app by the env var. IMO it is rather unusual for desktop apps, and I think it is better to add a config entry for this.
You can take a look at Configuration and appConfigSchema.ts to see how we configure other things.

Joerger · 2023-08-09T20:58:18Z

@ravicious @gzdunek For some reason, this OnApprove call results in an infinite loop, sending countless headless approval requests to the tshd daemon and overloading the yubikey. I have no idea why this would be an infinite loop while the on button press below works perfectly fine. Any ideas?

That's because the body of a component function, HeadlessPrompt, is executed on each render and a component gets rendered each time you update it's state. So you end up with an infinite loop.

Try this patch:

Patch

diff --git a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx index 08da0c1aa6..fc73d9b1c3 100644 --- a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx +++ b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useRef } from 'react'; +import React, { useRef, useEffect } from 'react'; import { useAsync } from 'shared/hooks/useAsync'; @@ -69,6 +69,12 @@ export function HeadlessAuthentication(props: HeadlessAuthenticationProps) { } } + useEffect(() => { + if (props.skipConfirm && updateHeadlessStateAttempt.status === '') { + handleHeadlessApprove(); + } + }, []); + return ( <HeadlessPrompt cluster={cluster} diff --git a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx index f1da2f1bd8..b0294cb162 100644 --- a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx +++ b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx @@ -52,13 +52,9 @@ export function HeadlessPrompt({ updateHeadlessStateAttempt, onCancel, }: HeadlessPromptProps) { - const [waitForMfa, setWaitForMfa] = useState(false); - - // skip to MFA confirmation step. - if (skipConfirm) { - setWaitForMfa(true); - onApprove(); - } + // skipConfirm automatically attempts to approve a headless auth attempt, + // so let's show waitForMfa from the very beginning in that case. + const [waitForMfa, setWaitForMfa] = useState(skipConfirm); return ( <DialogConfirmation

Ah I suspected it was something like that but had no idea how to fix it, thanks!

ravicious

It should be good to go after implementing the changes I mentioned.

ravicious · 2023-08-10T10:40:55Z

That's because the body of a component function, HeadlessPrompt, is executed on each render and a component gets rendered each time you update it's state. So you end up with an infinite loop.

Try this patch:

Patch

diff --git a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx index 08da0c1aa6..fc73d9b1c3 100644 --- a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx +++ b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useRef } from 'react'; +import React, { useRef, useEffect } from 'react'; import { useAsync } from 'shared/hooks/useAsync'; @@ -69,6 +69,12 @@ export function HeadlessAuthentication(props: HeadlessAuthenticationProps) { } } + useEffect(() => { + if (props.skipConfirm && updateHeadlessStateAttempt.status === '') { + handleHeadlessApprove(); + } + }, []); + return ( <HeadlessPrompt cluster={cluster} diff --git a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx index f1da2f1bd8..b0294cb162 100644 --- a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx +++ b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessPrompt/HeadlessPrompt.tsx @@ -52,13 +52,9 @@ export function HeadlessPrompt({ updateHeadlessStateAttempt, onCancel, }: HeadlessPromptProps) { - const [waitForMfa, setWaitForMfa] = useState(false); - - // skip to MFA confirmation step. - if (skipConfirm) { - setWaitForMfa(true); - onApprove(); - } + // skipConfirm automatically attempts to approve a headless auth attempt, + // so let's show waitForMfa from the very beginning in that case. + const [waitForMfa, setWaitForMfa] = useState(skipConfirm); return ( <DialogConfirmation

…al state. This makes it possible to reject a headless authentication when it skips the initial confirmation step.

gzdunek

Tested, works fine.

public-teleport-github-review-bot · 2023-08-14T19:50:50Z

@Joerger See the table below for backport results.

Branch	Result
branch/v13	Failed

* Support TELEPORT_HEADLESS_SKIP_CONFIRM env flag in teleport connect. * Replace reject button with cancel button that persists through approval state. This makes it possible to reject a headless authentication when it skips the initial confirmation step. * Add headlessSkipConfirm config option. * Apply changes from CR.

Joerger requested review from gzdunek and ravicious August 1, 2023 18:55

github-actions Bot added size/lg ui labels Aug 1, 2023

github-actions Bot requested review from fspmarshall, rudream and ryanclark August 1, 2023 18:56

Joerger removed request for fspmarshall, rudream and ryanclark August 1, 2023 18:57

Joerger force-pushed the joerger/headless-approval-teleport-connect-skip-confirm branch from 21e0415 to ffd0692 Compare August 1, 2023 20:05

Joerger mentioned this pull request Aug 1, 2023

[v13] Add Headless Polling to Teleport Connect #28975

Merged

Joerger added the backport/branch/v13 label Aug 1, 2023

Joerger force-pushed the joerger/headless-approval-teleport-connect-skip-confirm branch from ffd0692 to c15597c Compare August 1, 2023 20:30

ravicious added size/sm and removed size/lg labels Aug 2, 2023

Joerger marked this pull request as draft August 3, 2023 17:34

Joerger force-pushed the joerger/headless-approval-teleport-connect-skip-confirm branch from b32bd00 to b9cd730 Compare August 9, 2023 20:56

Joerger commented Aug 9, 2023

View reviewed changes

ravicious approved these changes Aug 10, 2023

View reviewed changes

Joerger added 4 commits August 10, 2023 11:49

Support TELEPORT_HEADLESS_SKIP_CONFIRM env flag in teleport connect.

1975b54

Replace reject button with cancel button that persists through approv…

5633daa

…al state. This makes it possible to reject a headless authentication when it skips the initial confirmation step.

Add headlessSkipConfirm config option.

0d9f770

Apply changes from CR.

c64fdd1

Joerger force-pushed the joerger/headless-approval-teleport-connect-skip-confirm branch from 885be29 to c64fdd1 Compare August 10, 2023 18:51

Joerger marked this pull request as ready for review August 10, 2023 18:52

github-actions Bot requested review from rudream and ryanclark August 10, 2023 18:52

Joerger mentioned this pull request Aug 10, 2023

Add Teleport Connect to Headless docs #30300

Merged

gzdunek approved these changes Aug 11, 2023

View reviewed changes

public-teleport-github-review-bot Bot removed request for rudream and ryanclark August 11, 2023 06:19

Joerger added this pull request to the merge queue Aug 14, 2023

Merged via the queue into master with commit d7ed907 Aug 14, 2023

Joerger deleted the joerger/headless-approval-teleport-connect-skip-confirm branch August 14, 2023 19:49

Joerger mentioned this pull request Aug 15, 2023

[v13] Teleport Connect headless approval - skipConfirm option #30475

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Teleport Connect headless approval - Skip Confirmation#29875

Teleport Connect headless approval - Skip Confirmation#29875
Joerger merged 4 commits intomasterfrom
joerger/headless-approval-teleport-connect-skip-confirm

Joerger commented Aug 1, 2023 •

edited

Loading

Uh oh!

Joerger commented Aug 1, 2023 •

edited

Loading

Uh oh!

gzdunek commented Aug 2, 2023

Uh oh!

Joerger Aug 9, 2023 •

edited

Loading

Uh oh!

ravicious Aug 10, 2023

Uh oh!

Joerger Aug 10, 2023

Uh oh!

ravicious left a comment

Uh oh!

Uh oh!

Uh oh!

ravicious Aug 10, 2023

Uh oh!

gzdunek left a comment

Uh oh!

public-teleport-github-review-bot Bot commented Aug 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Joerger commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Joerger commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gzdunek commented Aug 2, 2023

Uh oh!

Joerger Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravicious Aug 10, 2023

Choose a reason for hiding this comment

Uh oh!

Joerger Aug 10, 2023

Choose a reason for hiding this comment

Uh oh!

ravicious left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ravicious Aug 10, 2023

Choose a reason for hiding this comment

Uh oh!

gzdunek left a comment

Choose a reason for hiding this comment

Uh oh!

public-teleport-github-review-bot Bot commented Aug 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Joerger commented Aug 1, 2023 •

edited

Loading

Joerger commented Aug 1, 2023 •

edited

Loading

Joerger Aug 9, 2023 •

edited

Loading